Repository navigation
Conversation
Next Steps to MergeNext steps that must be taken to merge this PR:
Comment generated by summarize-checks workflow run. |
TypeSpec suppressions requiring reviewStatus: ✅ Approved — 4 suppressions This PR adds or updates the TypeSpec suppressions listed below. Suppressions are strongly discouraged — they bypass linter rules that protect API quality and consistency. Authors should avoid adding new suppressions and prefer fixing the underlying issue; reviewers should approve only when there is a clear, compelling justification and no reasonable alternative. Review each linked rule and source location, then apply New suppressions (4)
For an overview of TypeSpec linting rules click here. 💬 Have feedback on the TypeSpec suppression flow? Let us know. |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews
Comment generated by After APIView workflow run. |
…-virtualconnections-2026-09-01 Co-authored-by: cmriachi <104598291+cmriachi@users.noreply.github.com>
Co-authored-by: cmriachi <104598291+cmriachi@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ARM API Review SummaryReviewed PR #45406 at head SHA Caution Independent Critic verification did not run. The findings below are unverified. Severity is unchanged, and no ARM review labels were modified. Scoped review: 7 of 719 changed Approval labels observed:
No issues found; remaining concerns are covered by existing human-authored discussions and were not duplicated. posted-by: arm-api-reviewer-agent | rule: summary | category: summary | severity: suggestion | classification: existing | critic: unknown | head-sha: 89fef84
|
There was a problem hiding this comment.
ARM API Review
Posting findings from the ARM API Reviewer agent (Critic unavailable; reviewer self-check only, 3 iterations, no findings) against commit 89fef842b92db86604c1dd3dda6aa11a3d23f0f8. All findings are reported in the summary comment.
Approval labels observed: BreakingChange-Approved-BugFix, BreakingChange-Python-Sdk.
posted-by: arm-api-reviewer-agent | rule: summary | category: summary | severity: suggestion | classification: existing | critic: unknown | head-sha: 89fef84
🔍 ARM API review by ARM API Review: Automated Workflow
|
Check errors from Swagger Lintdiff. Use of additionalProperties is not allowed for properties owned by the service. The only time its ok to use it is when the properties are pass thru (user defined) and not subject to any validations. Please make this an array or provide an explanation for why you need this. |
|
Please fix lintdiff |
|
Pallavi Sah (@psah434) the lintdiff errors mentioned are related to old changes i did not introduce in this pr - not sure why they are detected as my changes - how do i fix that? |
|
|
||
| /** Where to resolve settings from. */ | ||
| #suppress "@azure-tools/typespec-azure-core/no-closed-literal-union" "The 2026-09-01 service rejects unknown virtual connection setting sources, so this union intentionally models a closed set." | ||
| @added(Versions.v2026_09_01) |
There was a problem hiding this comment.
If this enumeration will never add new values in a new api-version, please assert that in the justification. If it might add new values in new api-versions in the future, then it should be an open literal union (meaning you should add string as one of the variants.
|
|
||
| /** Protocol for the backend service. */ | ||
| #suppress "@azure-tools/typespec-azure-core/no-closed-literal-union" "The 2026-09-01 service rejects unsupported virtual connection protocols, so this union intentionally models a closed set." | ||
| @added(Versions.v2026_09_01) |
There was a problem hiding this comment.
If this enumeration will never add new values in a new api-version, please assert that in the justification. If it might add new values in new api-versions in the future, then it should be an open literal union (meaning you should add string as one of the variants.
|
|
||
| /** Authentication strategy for upstream connections. */ | ||
| #suppress "@azure-tools/typespec-azure-core/no-closed-literal-union" "The 2026-09-01 service rejects unsupported virtual connection authentication types, so this union intentionally models a closed set." | ||
| @added(Versions.v2026_09_01) |
There was a problem hiding this comment.
If this enumeration will never add new values in a new api-version, please assert that in the justification. If it might add new values in new api-versions in the future, then it should be an open literal union (meaning you should add string as one of the variants.
| /** The role a setting plays in a connection. */ | ||
| #suppress "@azure-tools/typespec-azure-core/no-closed-literal-union" "The 2026-09-01 service rejects unsupported virtual connection setting roles, so this union intentionally models a closed set." | ||
| @added(Versions.v2026_09_01) | ||
| union VirtualConnectionSettingRole { |
There was a problem hiding this comment.
If this enumeration will never add new values in a new api-version, please assert that in the justification. If it might add new values in new api-versions in the future, then it should be an open literal union (meaning you should add string as one of the variants.
There was a problem hiding this comment.
ARM API Review
Reviewed PR #45406 at head SHA 67493153cec1af143222d85a7f4d03c450e9a650 against base SHA a3be4b61f3eba3138075131e2669ab43e7841eaf.
Scoped review: This PR changes 719 files (+123,708/-405), exceeding the scoped-review cap. I reviewed the generated stable/2026-09-01/openapi.json, 49 representative stable/2026-09-01 examples, and the linked TypeSpec/configuration/suppression sources (EgressConfig.tsp, main.tsp, back-compatible.tsp, tspconfig.yaml, service.yaml, and readme.md). The remaining changed examples and lower-priority generated artifacts were excluded from this scoped pass.
Approval labels observed: BreakingChange-Approved-BugFix, BreakingChange-Python-Sdk. No Approved-Suppression or Approved-TypeSpecSuppression label was observed.
No net-new Blocking, Warning, or Suggestion findings are posted. Existing human-authored discussion already covers the current DELETE, dictionary-suppression, enum/open-union, and identityScope concerns; previously reported workflow findings were verified fixed at this SHA (ordinary singleton reads use GET, identityResourceId uses armResourceIdentifier, enum values are PascalCase, and relevant source contains no raw @extension).
Outcome: No additional ARM API issues were identified within the reviewed scope; existing human-authored findings remain unchanged.
ARM API review telemetry: scoped review; findings posted: 0 blocking, 0 warnings, 0 suggestions; critic verification attempted.
🔍 ARM API review by ARM API Review: Automated Workflow
ARM API Review SummaryReviewed PR #45406 at head SHA Scoped review: 50 of 719 changed Approval labels observed:
No net-new ARM API issues were identified within the reviewed scope. Previously reported singleton-read, TypeSpec extension, ARM resource-ID, and point-GET/suppression issues were verified fixed; existing human-authored discussions remain covered without duplicate findings. posted-by: arm-api-reviewer-agent | rule: summary | category: summary | severity: suggestion | classification: existing | critic: pass | head-sha: 1f46498
|
There was a problem hiding this comment.
ARM API Review
Reviewed PR #45406 at head SHA 1f464980bf761f1eebed95532f109ae94abec801 | Triggered by: pull_request_target
Scoped review: 50 of 719 changed specification/ files reviewed (PR exceeds the automated-review size cap). Not reviewed: the remaining generated artifacts, examples, and lower-priority specification files outside the prioritized scope.
Approval labels observed: BreakingChange-Approved-BugFix.
| Category | Count |
|---|---|
| 🔴 Blocking | 0 |
| 🟠 Warning | 0 |
| 🔵 Suggestion | 0 |
No net-new ARM API issues were identified within the reviewed scope. Previously reported singleton-read, TypeSpec extension, ARM resource-ID, and point-GET/suppression issues were verified fixed; existing human-authored discussions remain covered without duplicate findings.
posted-by: arm-api-reviewer-agent | rule: summary | category: summary | severity: suggestion | classification: existing | critic: pass | head-sha: 1f46498
🔍 ARM API review by ARM API Review: Automated Workflow
Ravi Eda (ravimeda)
left a comment
There was a problem hiding this comment.
ARM API Review
Posting findings from the ARM API Reviewer agent (critic-verified, 2 iterations, converged) against commit 1f46498. See inline comments for findings 1-4.
Approval labels observed: BreakingChange-Approved-BugFix.
| @@ -21,14 +20,14 @@ options: | |||
| emit-lro-options: "all" | |||
| examples-dir: "{project-root}/examples" | |||
| xml-strategy: "none" | |||
| use-read-only-status-schema: true | |||
There was a problem hiding this comment.
[NEW] 🔴 Blocking [Section 2 — API Versioning / ReadonlyPropertyChanged] specification/web/resource-manager/Microsoft.Web/AppService/tspconfig.yaml — line 23 — enabling use-read-only-status-schema changes published schemas in place.
Classification reasoning: This PR adds the project-wide emitter option. Against the pinned base, it changes 28 properties from writable to read-only in each of five existing API-version files—140 ReadonlyPropertyChanged errors total:
preview/2026-03-01-previewstable/2025-03-01stable/2025-05-01stable/2026-03-15stable/2026-07-15
Approval context: BreakingChange-Approved-BugFix is present. Confirm that its approval covers all 140 changes produced by this option, then resolve this conversation. If it does not, obtain matching breaking-change approval or avoid rewriting the published versions.
Suggested fix: Preserve use-read-only-status-schema only if the existing approval explicitly covers this complete project-wide output change. Otherwise remove or isolate the change and use the repository breaking-change process before regenerating these five OpenAPI files.
| @@ -0,0 +1,109 @@ | |||
| { | |||
| "operationId": "EgressConfigs_CreateOrUpdateEgressConfig", | |||
There was a problem hiding this comment.
[NEW] 🟠 Warning [EX-ORPHAN] / [EX-COVERAGE] specification/web/resource-manager/Microsoft.Web/AppService/examples/2026-09-01/EgressConfig_CreateOrUpdate.json — line 2 — all eight site PUT examples use an operation ID that does not exist in the emitted OpenAPI.
Classification reasoning: The Egress site PUT operation and these examples are introduced in this PR; neither exists in stable/2026-07-15.
Issue: The eight EgressConfig_CreateOrUpdate*.json/EgressConfig_Disable.json files declare EgressConfigs_CreateOrUpdateEgressConfig, while the emitted operation at stable/2026-09-01/openapi.json line 10054 is WebApps_CreateOrUpdateEgressConfig. All eight source files and their generated copies are orphaned, and the PUT operation has no x-ms-examples entry.
Suggested fix: Change line 2 in all eight source examples to "operationId": "WebApps_CreateOrUpdateEgressConfig", regenerate the stable examples/OpenAPI, and verify that the PUT operation contains all intended x-ms-examples references.
| @@ -0,0 +1,74 @@ | |||
| { | |||
| "operationId": "EgressConfigSlotConfigs_CreateOrUpdateEgressConfigSlot", | |||
There was a problem hiding this comment.
[NEW] 🟠 Warning [EX-ORPHAN] / [EX-COVERAGE] specification/web/resource-manager/Microsoft.Web/AppService/examples/2026-09-01/SlotEgressConfig_CreateOrUpdate.json — line 2 — both slot PUT examples use an operation ID that does not exist in the emitted OpenAPI.
Classification reasoning: The Egress slot PUT operation and these examples are introduced in this PR; neither exists in stable/2026-07-15.
Issue: The two SlotEgressConfig_CreateOrUpdate*.json files declare EgressConfigSlotConfigs_CreateOrUpdateEgressConfigSlot, while the emitted operation at stable/2026-09-01/openapi.json line 22804 is WebApps_CreateOrUpdateEgressConfigSlot. Both source files and generated copies are orphaned, and the slot PUT operation has no x-ms-examples entry.
Suggested fix: Change line 2 in both source examples to "operationId": "WebApps_CreateOrUpdateEgressConfigSlot", regenerate, and verify that the slot PUT operation contains both x-ms-examples references.
| #suppress "@azure-tools/typespec-azure-core/no-legacy-usage" "FIXME: Update justification, follow aka.ms/tsp/conversion-fix for details" | ||
| @@flattenProperty(SiteAuthSettingsV2.properties); | ||
|
|
||
| @@clientLocation(EgressConfigs.createOrUpdateEgressConfig, WebApps, "!csharp"); |
There was a problem hiding this comment.
[NEW] 🟠 Warning [TypeSpec Section 4.6 — Client Customizations] specification/web/resource-manager/Microsoft.Web/AppService/back-compatible.tsp — lines 2088-2099 — the four new Egress @@clientLocation customizations are placed outside client.tsp.
Classification reasoning: These customizations target the four new Egress operations and are added by this PR; the operations and decorators do not exist at the base SHA.
Issue: Section 4.6 requires client-level customizations to be placed in client.tsp and prohibits importing or using @azure-tools/typespec-client-generator-core in other files. This project already has client.tsp, but the new decorators were appended to back-compatible.tsp.
Suggested fix: Move the four Egress @@clientLocation decorators to client.tsp, remove them from back-compatible.tsp, regenerate, and revalidate the example operation IDs against the emitted OpenAPI.
Remove the project-wide use-read-only-status-schema option and regenerate all six OpenAPI versions without changing property-level visibility. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 119d5772-2836-4eb4-9579-8135879d42af
Link the site and slot PUT examples to their WebApps operation IDs, move the four Egress client customizations into client.tsp, and correct the DELETE-suppression rationale to describe the supported PUT-clear contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 119d5772-2836-4eb4-9579-8135879d42af
ARM API Review SummaryReviewed SHA: Scoped review: 8 of 714 changed files were reviewed because this PR exceeds the automated full-review threshold. Reviewed surfaces were
No net-new ARM API issues were identified within the reviewed scope. Existing human-authored discussions already cover DELETE behavior, typed dictionaries/Record suppression, enum/open-union behavior, Critic status: Independent critic verification completed successfully, with 1 iteration and convergence. No labels were changed. Exact approval-label inventory:
|
There was a problem hiding this comment.
ARM API Review
Reviewed PR #45406 at pinned head SHA 9fb22e26994d7005f5f44d4f52efa4fd0de86c59 against base SHA f1f796502a81c1cf4d44fe97109e8a71fe8ba485.
Scoped review: 8 of 714 changed files were reviewed because the PR exceeds the automated full-review threshold. The scope covered the highest-risk TypeSpec, configuration, suppression, service metadata, and generated 2026-09-01 OpenAPI surfaces. Remaining generated examples and lower-priority generated artifacts were not individually reviewed.
No net-new Blocking, Warning, or Suggestion findings were identified in scope. Existing human-authored discussions covering DELETE behavior, typed dictionaries/Record suppression, enum/open-union behavior, identityScope URI typing, nullable connectionName, and suppression approval were not duplicated or altered. Previously reported workflow findings for singleton POST reads, point GET coverage, identityResourceId, raw TypeSpec extensions, and stale emitter pins were verified fixed or superseded at this SHA.
Critic verification: independent critic completed successfully; 1 review iteration, converged. Approval-label inventory at final metadata check: BreakingChange-Approved-BugFix, BreakingChange-Python-Sdk; no Versioning-Approved-*, Approved-Suppression, or Approved-TypeSpecSuppression labels.
🔍 ARM API review by ARM API Review: Automated Workflow
|
/arm-review |
ARM API Review SummaryReviewed PR #45406 at head SHA Scoped review: 50 of 714 changed Approval labels observed:
No issues found in the scoped review. Existing discussions cover prior candidate concerns, and no new independently anchorable violation was identified at the pinned head. posted-by: arm-api-reviewer-agent | rule: summary | category: summary | severity: suggestion | classification: existing | critic: pass | head-sha: 9fb22e2
|
There was a problem hiding this comment.
ARM API Review
Posting findings from the ARM API Reviewer agent (critic-verified, 3 iterations, no new findings) against commit 9fb22e26994d7005f5f44d4f52efa4fd0de86c59. All findings are reported in the summary comment.
Approval labels observed: BreakingChange-Approved-BugFix.
posted-by: arm-api-reviewer-agent | rule: summary | category: summary | severity: suggestion | classification: existing | critic: pass | head-sha: 9fb22e2
🔍 ARM API review by ARM API Review: Automated Workflow
Ravi Eda (ravimeda)
left a comment
There was a problem hiding this comment.
ARM API Review
Posting findings from the ARM API Reviewer agent (critic-verified, 3 iterations, converged) against commit 9fb22e2. See inline comments for finding 1.
Approval labels observed: BreakingChange-Approved-BugFix.
| @@ -100,6 +100,54 @@ directive: | |||
| - $.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Web/sites/{name}/slots/{slot}/instances/{instanceId}/extensions/MSDeploy"] | |||
| - $.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Web/sites/{name}/slots/{slot}/instances/{instanceId}/extensions/MSDeploy/log"] | |||
| reason: MSDeploy is the intentional name matching the existing service API. | |||
| - suppress: EvenSegmentedPathForPutOperation | |||
There was a problem hiding this comment.
[NEW] 🔵 Suggestion — RPC-SUPPRESS-SCOPE — readme.md:103, also lines 111, 119 and 132: Consider version-binding the new Egress waivers.
All four directives are in the unconditional YAML block beginning at line 42. Their precise where clauses correctly limit the affected elements, but from: openapi.json has no API-version boundary. Today these targets occur only in 2026-09-01; future packages retaining the same paths/model names would inherit the waivers. This is optional scope hardening—not a claim that earlier versions are currently affected or that global directives are categorically forbidden.
Classification reasoning: These four directives are new in this PR. The 503 existing occurrences are outside this recommendation.
Approval context: BreakingChange-Approved-BugFix is present; Approved-Suppression is absent. Breaking-change approval does not authorize these Egress suppressions or their future-version carryover. If a PR-level suppression approval is obtained, please confirm that it covers these four directives and their unconditional scope; when it does, the remaining action is to resolve this conversation. Otherwise, consider version-binding them as suggested.
Suggested fix: If retained, place only these four entries in the conditional package-2026-09 block, or use equivalent version-specific from matching. Preserve the exact JSONPaths and existing directives. Do not delete error suppressions without addressing their still-firing diagnostics.
Purpose of this PR
What's the purpose of this PR? Check the specific option that applies. This is mandatory!
Due diligence checklist
To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:
ARM resource provider contract and
REST guidelines (estimated time: 4 hours).
I understand this is required before I can proceed to the diagram Step 2, "ARM API changes review", for this PR.